feat(gitlab): Cache results acl project membership #2280

chmouel · 2025-10-10T09:52:18Z

Implemented caching in gitlab for results returned by checkMembership.
This was done to reduce repeated calls to the GitLab API when checking
the membership status of the same user multiple times during processing
of an event.

Co-authored-by: Claude [email protected]
Signed-off-by: Chmouel Boudjnah [email protected]

📝 Description of the Change

👨🏻‍ Linked Jira

Jira: https://issues.redhat.com/browse/SRVKP-9056

🔗 Linked GitHub Issue

Fixes #

🚀 Type of Change

🧪 Testing Strategy

🤖 AI Assistance

I have not used any AI assistance for this PR.
I have used AI assistance for this PR.

If you have used AI assistance, please provide the following details:

Which LLM was used?

Extent of AI Assistance:

Documentation and research only
Unit tests or E2E tests only
Code generation (parts of the code)
Full code generation (most of the PR)
PR description and comments
Commit message(s)

Important

If the majority of the code in this PR was generated by an AI, please add a Co-authored-by trailer to your commit message.
For example:

Co-authored-by: Gemini [email protected]
Co-authored-by: ChatGPT [email protected]
Co-authored-by: Claude [email protected]
Co-authored-by: Cursor [email protected]
Co-authored-by: Copilot [email protected]

**💡You can use the script ./hack/add-llm-coauthor.sh to automatically add
these co-author trailers to your commits.

✅ Submitter Checklist

gemini-code-assist · 2025-10-10T09:52:33Z

Summary of Changes

Hello @chmouel, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a significant performance optimization for GitLab integration by implementing a caching layer for project membership checks. By storing the results of checkMembership calls, the system can avoid repeated and potentially costly API requests to GitLab, especially when processing events that involve multiple checks for the same user. This change aims to improve the responsiveness and reduce the load on the GitLab API, leading to a more efficient and scalable solution.

Highlights

GitLab Membership Caching: Implemented a caching mechanism for GitLab project membership checks (checkMembership) to reduce redundant API calls to the GitLab server.
Lazy Cache Initialization: The memberCache is now lazily initialized within the checkMembership function, ensuring it's only created when needed.
Error Handling for Caching: Modified the caching logic to explicitly not cache results when the GitLab API call fails, allowing for retries on transient network or API issues.
New Test Cases: Added comprehensive test cases (TestMembershipCaching and TestMembershipAPIFailureDoesNotCacheFalse) to verify the correct behavior of the caching mechanism, including scenarios for successful caching and API failures.
Testing Utility: Introduced a new helper function MuxAllowUserIDCounting in the test utilities to facilitate counting API calls during tests, enabling robust verification of caching efficiency.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Copilot

Pull Request Overview

This PR implements caching functionality for GitLab project membership checks to reduce redundant API calls when verifying the same user's membership status multiple times during event processing.

Key changes:

Added a memberCache map to the GitLab Provider struct to cache membership results by user ID
Modified the checkMembership method to check cache first and store results for successful API calls
Implemented logic to avoid caching API failures to allow retry on transient errors

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
pkg/provider/gitlab/gitlab.go	Added memberCache field to Provider struct with documentation
pkg/provider/gitlab/acl.go	Implemented caching logic in checkMembership method with cache initialization and result storage
pkg/provider/gitlab/test/test.go	Added helper function for testing caching behavior with API call counting
pkg/provider/gitlab/acl_test.go	Added comprehensive tests for membership caching and failure handling scenarios

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

pkg/provider/gitlab/acl.go

gemini-code-assist

Code Review

This pull request introduces caching for GitLab project membership checks to reduce API calls, which is a great performance improvement. The implementation is mostly solid, with good test coverage for the happy path and for uncached failures. However, I've found a potential issue in the error handling logic where a successful fallback check during an API failure could lead to incorrect caching, preventing future API checks for that user. My review includes a suggestion to fix this.

pkg/provider/gitlab/acl.go

have some question

aThorp96 · 2025-10-10T12:05:39Z

reduce repeated calls to the GitLab API when checking the membership status of the same user multiple times during processing of an event.

What's the reason we make multiple call to check the membership multiple times when processing an event? If caching the first response works then only one call should be necessary. Reducing the call volume seems preferable, especially since the Provider object only lives for the duration of one event; it would only ever check one username FWICT

chmouel · 2025-10-10T12:29:26Z

What's the reason we make multiple call to check the membership multiple times when processing an event? If caching the first response works then only one call should be necessary. Reducing the call volume seems preferable, especially since the Provider object only lives for the duration of one event; it would only ever check one username FWICT

it's basically how the code is structured.. when we check from a comment we need to do a ACL Check from the submitted sending /ok-to-test with all the logic coming if allowed from repo/org or OWNERS files, it's a bit of a recursive function

chmouel · 2025-10-10T12:31:33Z

I do want in the future that we do some caching via TTL of git api fetch of objects when requested via SHA since those should be immutable, we have a jira story for it..

pkg/provider/gitlab/acl.go

pkg/provider/gitlab/acl_test.go

aThorp96 · 2025-10-10T12:43:17Z

pkg/provider/gitlab/acl_test.go

 	}
 }
+
+func TestMembershipCaching(t *testing.T) {


Can you also add a test to check the caching on

user is allowed via owners file

user is not allowed

weird that they were not added previously, i'll add a github issue so we can properly coverage acl.go in gitlab like the way we do in github

here is a jira ticket for it https://issues.redhat.com/browse/SRVKP-9074

Implemented caching in gitlab for results returned by `checkMembership`. This was done to reduce repeated calls to the GitLab API when checking the membership status of the same user multiple times during processing of an event. Co-authored-by: Claude <[email protected]> Jira: https://issues.redhat.com/browse/SRVKP-9056 Signed-off-by: Chmouel Boudjnah <[email protected]>

chmouel · 2025-10-27T07:14:14Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces caching for GitLab project membership checks, which is a great way to reduce redundant API calls and improve performance. The implementation is logical and includes new tests to verify the caching behavior, including how it handles API failures. My main feedback is to address a potential race condition in the cache implementation. The current code is not thread-safe, which could cause issues if used concurrently. I've provided suggestions to add a mutex and refactor the caching logic to ensure thread safety.

pkg/provider/gitlab/gitlab.go

pkg/provider/gitlab/acl.go

chmouel · 2025-10-27T13:10:18Z

Cross checked the gemini reviews that think we can have a unsafe concurrency access and it's not the case since we spin up our own coroutine in the adapter with its own context that would not access concurrently between request.

Copilot AI review requested due to automatic review settings October 10, 2025 09:52

Copilot AI reviewed Oct 10, 2025

View reviewed changes

pkg/provider/gitlab/acl.go Outdated Show resolved Hide resolved

pkg/provider/gitlab/acl.go Show resolved Hide resolved

pipelines-as-code bot added enhancement New feature or request gitlab optimization labels Oct 10, 2025

gemini-code-assist bot reviewed Oct 10, 2025

View reviewed changes

pkg/provider/gitlab/acl.go Show resolved Hide resolved

chmouel force-pushed the SRVKP-9056-feat-gitlab-cache-results-acl-project-membership branch from 3e6bad3 to af38bdb Compare October 10, 2025 10:00

zakisk previously approved these changes Oct 10, 2025

View reviewed changes

zakisk reviewed Oct 10, 2025

View reviewed changes

pkg/provider/gitlab/acl.go Show resolved Hide resolved

aThorp96 reviewed Oct 10, 2025

View reviewed changes

chmouel force-pushed the SRVKP-9056-feat-gitlab-cache-results-acl-project-membership branch from af38bdb to 731bea8 Compare October 16, 2025 12:39

zakisk approved these changes Oct 27, 2025

View reviewed changes

gemini-code-assist bot reviewed Oct 27, 2025

View reviewed changes

pkg/provider/gitlab/gitlab.go Show resolved Hide resolved

pkg/provider/gitlab/acl.go Show resolved Hide resolved

chmouel merged commit e45ee14 into openshift-pipelines:main Oct 27, 2025
5 checks passed

chmouel deleted the SRVKP-9056-feat-gitlab-cache-results-acl-project-membership branch October 27, 2025 13:10

feat(gitlab): Cache results acl project membership #2280

feat(gitlab): Cache results acl project membership #2280

Uh oh!

Conversation

chmouel commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 Description of the Change

👨🏻‍ Linked Jira

🔗 Linked GitHub Issue

🚀 Type of Change

🧪 Testing Strategy

🤖 AI Assistance

✅ Submitter Checklist

Uh oh!

gemini-code-assist bot commented Oct 10, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

aThorp96 commented Oct 10, 2025

Uh oh!

chmouel commented Oct 10, 2025

Uh oh!

chmouel commented Oct 10, 2025

Uh oh!

Uh oh!

Uh oh!

aThorp96 Oct 10, 2025

Choose a reason for hiding this comment

Uh oh!

chmouel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

chmouel Oct 14, 2025

Choose a reason for hiding this comment

Uh oh!

chmouel commented Oct 27, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

chmouel commented Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

chmouel commented Oct 10, 2025 •

edited

Loading